NO-ISSUE: Fix resource leak and inconsistency in manifest download by zaneb · Pull Request #10322 · openshift/assisted-service

zaneb · 2026-05-12T21:02:13Z

Fix two issues identified by Claude Code in createClusterDataFiles():

Close the io.ReadCloser returned by Download() to prevent resource leaks when processing multiple manifests
Use the objectHandler parameter consistently instead of mixing m.objectHandler and objectHandler for different operations

The response body is closed immediately after io.ReadAll() rather than using defer, since this occurs in a loop and defer would accumulate all closes until function return.

Summary by CodeRabbit

Bug Fixes
- Fixed resource management for cluster manifest downloads from object storage, ensuring proper cleanup of download streams to prevent potential leaks.

Fix two issues in createClusterDataFiles(): 1. Close the io.ReadCloser returned by Download() to prevent resource leaks when processing multiple manifests 2. Use the objectHandler parameter consistently instead of mixing m.objectHandler and objectHandler for different operations The response body is closed immediately after io.ReadAll() rather than using defer, since this occurs in a loop and defer would accumulate all closes until function return. Assisted-by: Claude Code

openshift-ci-robot · 2026-05-12T21:02:18Z

@zaneb: This pull request explicitly references no jira issue.

Details

In response to this:

Fix two issues identified by Claude Code in createClusterDataFiles():

Close the io.ReadCloser returned by Download() to prevent resource leaks when processing multiple manifests

Use the objectHandler parameter consistently instead of mixing m.objectHandler and objectHandler for different operations

The response body is closed immediately after io.ReadAll() rather than using defer, since this occurs in a loop and defer would accumulate all closes until function return.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

coderabbitai · 2026-05-12T21:02:30Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 67a80e96-13ff-4ade-9376-48e5570f8ea9

📥 Commits

Reviewing files that changed from the base of the PR and between babc11d and 6347b5c.

📒 Files selected for processing (1)

internal/cluster/cluster.go

Walkthrough

The createClusterDataFiles function in cluster.go now uses the passed-in objectHandler parameter for manifest downloads instead of the receiver's member field, and explicitly closes the download response after reading file content for proper resource cleanup.

Changes

Manifest Download Resource Management

Layer / File(s)	Summary
Manifest download and response cleanup `internal/cluster/cluster.go`	Updated manifest file download to use the passed-in `objectHandler` parameter instead of `m.objectHandler`, and added explicit `response.Close()` after reading file content to ensure S3 stream resources are properly released.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error)

Check name	Status	Explanation	Resolution
Stable And Deterministic Test Names	❌ Error	Test at internal/cluster/cluster_test.go:4413 uses fmt.Sprintf to construct test name dynamically: It(fmt.Sprintf("success for stage '%s'", stage), func(). Violates requirement for stable test names.	Use static test name instead of fmt.Sprintf. Replace with It("successfully updates finalizing stage") and move stage-specific logic to test body.

✅ Passed checks (11 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main changes: fixing a resource leak and inconsistency in manifest downloading, which matches the file-level summary and PR objectives.
Description check	✅ Passed	The description addresses the required template sections with meaningful content about the bug fix, testing approach, and implementation rationale, though some checklist items remain incomplete.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Test Structure And Quality	✅ Passed	Check not applicable. PR only modifies production code (internal/cluster/cluster.go); no Ginkgo test files are changed. The custom check specifically targets test code review.
Microshift Test Compatibility	✅ Passed	This PR does not add any Ginkgo e2e tests. It modifies backend code in internal/cluster/cluster.go. The check is not applicable.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	This PR does not add any Ginkgo e2e tests. It fixes a resource leak in internal/cluster/cluster.go. The SNO Test Compatibility check only applies when new Ginkgo tests are added.
Topology-Aware Scheduling Compatibility	✅ Passed	PR fixes resource leaks in internal log collection utility. Does not modify deployment manifests, operator code, or introduce scheduling constraints. Topology check not applicable.
Ote Binary Stdout Contract	✅ Passed	The PR modifies internal/cluster/cluster.go, a regular service package not a test file. No process-level entry points or stdout writes. The OTE Binary Stdout Contract check is inapplicable here.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	This PR only modifies internal/cluster/cluster.go to fix resource leaks. It does not add any new Ginkgo e2e tests, so the IPv6/disconnected test check is not applicable.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

Generate code and open pull requests
Plan features and break down work
Investigate incidents and troubleshoot customer tickets together
Automate recurring tasks and respond to alerts with triggers
Summarize progress and report instantly

Built for teams:

Shared memory across your entire org—no repeating context
Per-thread sandboxes to safely plan and execute work
Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

openshift-ci · 2026-05-12T21:02:44Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: zaneb
Once this PR has been reviewed and has the lgtm label, please assign adriengentil for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

codecov · 2026-05-12T21:27:49Z

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 44.33%. Comparing base (babc11d) to head (6347b5c).

Files with missing lines	Patch %	Lines
internal/cluster/cluster.go	0.00%	2 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #10322   +/-   ##
=======================================
  Coverage   44.33%   44.33%           
=======================================
  Files         417      417           
  Lines       72837    72838    +1     
=======================================
+ Hits        32294    32295    +1     
  Misses      37609    37609           
  Partials     2934     2934

Files with missing lines	Coverage Δ
internal/cluster/cluster.go	`66.43% <0.00%> (-0.06%)`	⬇️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

openshift-ci · 2026-05-13T00:55:35Z

@zaneb: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/edge-e2e-ai-operator-ztp	`6347b5c`	link	true	`/test edge-e2e-ai-operator-ztp`
ci/prow/edge-verify-generated-code	`6347b5c`	link	true	`/test edge-verify-generated-code`
ci/prow/verify-generated-code	`6347b5c`	link	true	`/test verify-generated-code`
ci/prow/edge-e2e-metal-assisted-4-22	`6347b5c`	link	true	`/test edge-e2e-metal-assisted-4-22`

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label May 12, 2026

openshift-ci Bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label May 12, 2026

openshift-ci Bot requested review from jhernand and tsorya May 12, 2026 21:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NO-ISSUE: Fix resource leak and inconsistency in manifest download#10322

NO-ISSUE: Fix resource leak and inconsistency in manifest download#10322
zaneb wants to merge 1 commit into
openshift:masterfrom
zaneb:log-download-close

zaneb commented May 12, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

openshift-ci-robot commented May 12, 2026

Uh oh!

coderabbitai Bot commented May 12, 2026 •

edited

Loading

Pre-merge checks failed

Uh oh!

openshift-ci Bot commented May 12, 2026

Uh oh!

codecov Bot commented May 12, 2026 •

edited

Loading

Uh oh!

openshift-ci Bot commented May 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

zaneb commented May 12, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

openshift-ci-robot commented May 12, 2026

Uh oh!

coderabbitai Bot commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Pre-merge checks failed

❌ Failed checks (1 error)

Uh oh!

openshift-ci Bot commented May 12, 2026

Uh oh!

codecov Bot commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

openshift-ci Bot commented May 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

zaneb commented May 12, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 12, 2026 •

edited

Loading

codecov Bot commented May 12, 2026 •

edited

Loading